refactor(grids): fix styling for pinned column border - #17583
Open
desig9stein wants to merge 8 commits into
Open
refactor(grids): fix styling for pinned column border#17583desig9stein wants to merge 8 commits into
desig9stein wants to merge 8 commits into
Conversation
… update CSS custom properties
…d add support for conditional width theming
…te functionality into column pinning sample
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new gridRowPinning redirect route is missing pathMatch: 'full', which can cause unintended prefix-matching redirects and is inconsistent with the existing redirect pattern in the same file.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR addresses inconsistent pinned-column border styling in the grid header by aligning header pinned borders with the grid theme’s pinned border tokens, and follows up with a breaking theming change that removes independent summary border width theming (with an accompanying ng update migration).
Changes:
- Fix pinned header border rendering so pinned borders consistently use the configured pinned border width/style/color.
- Remove
grid-summary-themeborder-width theming (and related CSS custom properties) so summary borders inherit width from the grid; add migration + tests. - Consolidate demo samples by folding row pinning into the column pinning sample and updating routes/navigation accordingly.
File summaries
| File | Description |
|---|---|
| src/app/properties-panel/properties-panel.component.html | Uses singleRequired selection mode for button-group controls. |
| src/app/grid-theme-builder/grid-theme-builder.sample.ts | Removes summary border width signals and makes border export resilient to missing width signals. |
| src/app/grid-theme-builder/grid-theme-builder.sample.html | Removes deprecated summary border width CSS custom property bindings. |
| src/app/grid-theme-builder/border-rule-editor.types.ts | Makes border width optional for targets that are no longer independently themable. |
| src/app/grid-theme-builder/border-rule-editor.component.ts | Updates border editing logic to tolerate missing width signals. |
| src/app/grid-theme-builder/border-rule-editor.component.html | Conditionally hides the border-width editor when width isn’t supported. |
| src/app/grid-row-pinning/grid-row-pinning.sample.ts | Removed sample (row pinning moved into column pinning sample). |
| src/app/grid-row-pinning/grid-row-pinning.sample.scss | Removed sample styles. |
| src/app/grid-row-pinning/grid-row-pinning.sample.html | Removed sample template. |
| src/app/grid-column-pinning/grid-column-pinning.sample.ts | Reworks sample to cover both column + row pinning across grid types with properties panel integration. |
| src/app/grid-column-pinning/grid-column-pinning.sample.scss | Updates sample layout/styles for the new combined pinning demo. |
| src/app/grid-column-pinning/grid-column-pinning.sample.html | New combined pinning demo UI (tabs + pinning actions + status). |
| src/app/app.routes.ts | Redirects gridRowPinning to the combined pinning sample route. |
| src/app/app.component.ts | Updates navigation label and removes separate row pinning nav entry. |
| projects/igniteui-angular/migrations/update-22_2_0_grid-summary-borders/index.ts | Adds migration to rename removed summary border-width CSS custom properties to grid equivalents. |
| projects/igniteui-angular/migrations/update-22_2_0_grid-summary-borders/index.spec.ts | Adds unit tests validating the migration behavior. |
| projects/igniteui-angular/migrations/update-22_2_0_grid-summary-borders/changes/theme-changes.json | Declares removal of $border-width and $pinned-border-width from grid-summary-theme. |
| projects/igniteui-angular/migrations/migration-collection.json | Registers the new 22.2.0 migration. |
| projects/igniteui-angular/grids/themes/_base.scss | Ensures pinned header borders use pinned border tokens consistently (including group titles). |
| projects/igniteui-angular/grids/core/src/summaries/themes/_base.scss | Makes summary border widths inherit from grid-provided internal CSS vars. |
| CHANGELOG.md | Documents the breaking change around summary border width theming and the available migration. |
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
…l-sided pinning configuration and simplify overrides logic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #17353
Related PR from theming: IgniteUI/igniteui-theming#605
Description
Motivation / Context
Type of Change (check all that apply):
Component(s) / Area(s) Affected:
How Has This Been Tested?
Test Configuration:
Screenshots / Recordings
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)